docs: replace hystrix with executor hook and failsafe-go examples#75
docs: replace hystrix with executor hook and failsafe-go examples#75
Conversation
…xamples Update all 5 pages that reference hystrix to document the new executor-based resilience API (interceptors.SetDefaultExecutor): - integrations.md: replace Hystrix-Go section with Circuit Breaker / Resilience section including code examples (simple CB, per-method, disable, excluded errors) - howto/Metrics.md: replace Hystrix Metrics section with Circuit Breaker Metrics showing failsafe-go event listener pattern - FAQ.md: update hystrixprometheus answer to point to executor API - Packages.md: mark hystrixprometheus as "will be archived in v1" - index.md: update features table to reference resilience executor hook
📝 WalkthroughWalkthroughDocumentation updates deprecate Changes
Possibly related PRs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Updates the ColdBrew docs to migrate resilience guidance away from Hystrix and toward the new executor-based hook (interceptors.SetDefaultExecutor) with failsafe-go examples.
Changes:
- Replaces Hystrix-focused content with an “executor hook” resilience approach and adds several failsafe-go circuit breaker examples.
- Updates metrics documentation to describe circuit breaker metrics as library-managed (vs ColdBrew-managed Hystrix metrics).
- Marks
hystrixprometheusas legacy/deprecated and updates cross-page references to the new integrations anchor.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| integrations.md | Replaces Hystrix section with executor-hook resilience docs + examples and new anchor for cross-linking. |
| howto/Metrics.md | Replaces Hystrix metrics section with failsafe-go listener-based circuit breaker metrics guidance. |
| Packages.md | Updates hystrixprometheus package description to deprecation + v1 archival note and points to executor hook. |
| Index.md | Adjusts feature table to remove “circuit breaker metrics” claim and point to pluggable executor hook. |
| FAQ.md | Updates hystrixprometheus answer to steer users to SetDefaultExecutor + failsafe-go and v1 status. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…, fix Playwright test
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
FAQ.md (1)
126-126: Optional: deep-link to the specific anchor for consistency.Other pages updated in this PR (
Packages.md,howto/Metrics.md) link to/integrations/#circuit-breaker--resilience. Using the same anchor here avoids sending readers to the top of a long page.✏️ Proposed diff
-ColdBrew now provides a pluggable executor hook (`interceptors.SetDefaultExecutor`) for circuit breaking. Use [failsafe-go](https://github.com/failsafe-go/failsafe-go) as the recommended resilience library. See the [integrations page](/integrations/) for setup examples. +ColdBrew now provides a pluggable executor hook (`interceptors.SetDefaultExecutor`) for circuit breaking. Use [failsafe-go](https://github.com/failsafe-go/failsafe-go) as the recommended resilience library. See the [integrations page](/integrations/#circuit-breaker--resilience) for setup examples.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@FAQ.md` at line 126, Update the integrations link in the FAQ.md line that mentions the pluggable executor hook (interceptors.SetDefaultExecutor) so it deep-links to the circuit breaker section; replace "/integrations/" with "/integrations/#circuit-breaker--resilience" so users land directly on the circuit-breaker--resilience anchor instead of the top of the integrations page.integrations.md (2)
296-316: Nit: lazy-construction under the lock is fine, but considersync.Mapor a pre-built map to avoid lock contention on the hot path.The current pattern acquires
muon every protected call just to look up (and, once, create) the breaker. For a hot client path this becomes a global contention point. Two simple alternatives:
- Pre-build all breakers once in
init()(sinceconfigsis static) and read from the map lock-free.- Use
sync.Mapor async.RWMutexwith a fast read path.♻️ Pre-built map variant
- var ( - mu sync.Mutex - breakers = make(map[string]circuitbreaker.CircuitBreaker[any]) - ) + breakers := make(map[string]circuitbreaker.CircuitBreaker[any], len(configs)) + for method, cfg := range configs { + breakers[method] = circuitbreaker.NewBuilder[any](). + WithFailureThreshold(cfg.failureThreshold). + WithDelay(cfg.delay). + Build() + } interceptors.SetDefaultExecutor(func(ctx context.Context, method string, fn func(ctx context.Context) error) error { - cfg, ok := configs[method] - if !ok { + cb, ok := breakers[method] + if !ok { return fn(ctx) // no circuit breaker for unconfigured methods } - - mu.Lock() - cb, exists := breakers[method] - if !exists { - cb = circuitbreaker.NewBuilder[any](). - WithFailureThreshold(cfg.failureThreshold). - WithDelay(cfg.delay). - Build() - breakers[method] = cb - } - mu.Unlock() - return failsafe.With[any](cb).WithContext(ctx).Run(func() error { return fn(ctx) }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations.md` around lines 296 - 316, The interceptor's hot path currently locks mu inside interceptors.SetDefaultExecutor to lookup/create breakers[method], causing contention; change to pre-build the breakers map once (using configs to construct each circuitbreaker via circuitbreaker.NewBuilder and store in breakers) before registering the executor so the executor can read breakers lock-free, or replace breakers+mu with a sync.Map (or use a sync.RWMutex with an RLock fast-path) and only write when creating a new breaker; update the executor so it only reads from breakers without holding mu and keeps the failsafe.With(...).Run usage unchanged.
242-354: Minor: code snippets omit imports.Across the four examples,
time,sync,context, andcodes(fromgoogle.golang.org/grpc/codes, used byWithExcludedCodes) are used without being shown in anyimport (...)block. For copy-paste friendliness, consider adding the imports to the first example or a short "imports used in this page" note near the top of the section.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations.md` around lines 242 - 354, The examples omit required imports (time, sync, context and grpc codes) which makes copy-paste fail; update the first example (or add a short "imports used in this page" note) to include the missing packages—add time, context, sync and google.golang.org/grpc/codes alongside the existing github.com/failsafe-go/failsafe-go, github.com/failsafe-go/failsafe-go/circuitbreaker and github.com/go-coldbrew/interceptors so all snippets (including functions init(), the per-method breakers using mu/breakers, and any usage of WithExcludedCodes) compile when copied.howto/Metrics.md (1)
142-150: Optional: clarify placeholder variables in the circuit breaker example.The snippet references
circuitStateGaugeandcommandNamewithout definition, and usestime.Secondwithout showing imports. A brief comment above the block (e.g. "circuitStateGaugeis a user-defined*prometheus.GaugeVec;commandNameidentifies the protected call") would help readers copy-paste the example with fewer surprises.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@howto/Metrics.md` around lines 142 - 150, Add a short explanatory comment above the circuit breaker example clarifying that circuitStateGauge is a user-defined *prometheus.GaugeVec used to export breaker state, commandName is the string identifying the protected call, and that time.Second requires importing the time package; leave the example code unchanged (symbols to reference: circuitStateGauge, commandName, time.Second, circuitbreaker.NewBuilder).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integrations.md`:
- Around line 242-270: The example using interceptors.SetDefaultExecutor and
related functions is invalid for go-coldbrew/interceptors; remove this executor
configuration block and replace it with a compile-able example that uses the
real public API: call interceptors.SetFilterFunc to mark protected methods (or a
filter closure), and register interceptors via
interceptors.UseColdBrewClientInterceptors or by composing
interceptors.DefaultInterceptors, then wrap the protected-method invocation with
your circuit-breaker logic (failsafe) inside the interceptor/filter you
register; update references to SetDefaultExecutor, WithoutExecutor,
WithExecutor, WithExcludedErrors, WithExcludedCodes, and
ExecutorClientInterceptor to use SetFilterFunc, UseColdBrewClientInterceptors,
and DefaultInterceptors instead so the snippet matches the package API.
- Around line 320-335: The documentation references non-existent API symbols
(WithoutExecutor, ExecutorClientInterceptor, WithExecutor, SetDefaultExecutor)
so update the "Disabling for specific connections" and "Per-call executor"
paragraphs to use the actual public interceptors (DefaultClientInterceptor,
DefaultClientInterceptors, HystrixClientInterceptor, NewRelicClientInterceptor)
and remove any example calling WithoutExecutor or ExecutorClientInterceptor;
state that the package does not expose per-call executor helpers and instead
explain the supported patterns (use DefaultClientInterceptors or compose
HystrixClientInterceptor/NewRelicClientInterceptor directly) and provide a
corrected, compiling example that uses DefaultClientInterceptor(s) only and a
note about how to implement per-call behavior by wiring a custom interceptor if
needed.
---
Nitpick comments:
In `@FAQ.md`:
- Line 126: Update the integrations link in the FAQ.md line that mentions the
pluggable executor hook (interceptors.SetDefaultExecutor) so it deep-links to
the circuit breaker section; replace "/integrations/" with
"/integrations/#circuit-breaker--resilience" so users land directly on the
circuit-breaker--resilience anchor instead of the top of the integrations page.
In `@howto/Metrics.md`:
- Around line 142-150: Add a short explanatory comment above the circuit breaker
example clarifying that circuitStateGauge is a user-defined *prometheus.GaugeVec
used to export breaker state, commandName is the string identifying the
protected call, and that time.Second requires importing the time package; leave
the example code unchanged (symbols to reference: circuitStateGauge,
commandName, time.Second, circuitbreaker.NewBuilder).
In `@integrations.md`:
- Around line 296-316: The interceptor's hot path currently locks mu inside
interceptors.SetDefaultExecutor to lookup/create breakers[method], causing
contention; change to pre-build the breakers map once (using configs to
construct each circuitbreaker via circuitbreaker.NewBuilder and store in
breakers) before registering the executor so the executor can read breakers
lock-free, or replace breakers+mu with a sync.Map (or use a sync.RWMutex with an
RLock fast-path) and only write when creating a new breaker; update the executor
so it only reads from breakers without holding mu and keeps the
failsafe.With(...).Run usage unchanged.
- Around line 242-354: The examples omit required imports (time, sync, context
and grpc codes) which makes copy-paste fail; update the first example (or add a
short "imports used in this page" note) to include the missing packages—add
time, context, sync and google.golang.org/grpc/codes alongside the existing
github.com/failsafe-go/failsafe-go,
github.com/failsafe-go/failsafe-go/circuitbreaker and
github.com/go-coldbrew/interceptors so all snippets (including functions init(),
the per-method breakers using mu/breakers, and any usage of WithExcludedCodes)
compile when copied.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d6d934d-3027-434d-823e-4de1cfe4d11c
📒 Files selected for processing (6)
FAQ.mdIndex.mdPackages.mdhowto/Metrics.mdintegrations.mdtests/content.spec.ts
Summary
interceptors.SetDefaultExecutor)Companion to go-coldbrew/interceptors#45 which adds the executor API.
Pages updated
integrations.mdhowto/Metrics.mdFAQ.mdPackages.mdindex.mdTest plan
/integrations/#circuit-breaker--resilience)Summary by CodeRabbit
Documentation
Tests